-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
REF: Consolidate validation of dictionary argument in agg/transform #40004
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@jreback - added test_invalid_arg to start reorganizing the apply tests to something with a better layout. If this seems like a good idea, will move other relevant tests here in a followup. |
great, this is a very small api change (e.g. now get SpecificationError rather than KeyError in some cases), can you add a whatsnew note. ping on green. |
@jreback Ah - thanks for catching this. Before adding the note - is it clear that we want a SpecificationError here and not KeyError? Looking throughout pandas.core, I only see SpecificationError used in three places:
In each case, I believe either KeyError or ValueError is also appropriate. It seems to me maybe changing to KeyError for transform here would be better, and removing SpecificationError altogether in the future. This would also having the benefit of a smaller API change (transform is only used by Series/DataFrame here, whereas agg is used by GroupBy/Resampler/Rolling). |
# GH 35964 | ||
obj = box({"A": [1]}) | ||
match = "nested renamer is not supported" | ||
with pytest.raises(SpecificationError, match=match): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this error is good
# GH 40004 | ||
obj = DataFrame({"A": [1]}) | ||
match = re.escape("Column(s) ['B'] do not exist") | ||
with pytest.raises(SpecificationError, match=match): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be ok here with a KeyError (which i think is the same as now)
…ply_validate_dict � Conflicts: � doc/source/whatsnew/v1.3.0.rst
very nice! |
Currently there are no tests for dict-like argument with agg when a DataFrame is missing a column (edit: in the apply tests, there are tests in groupby, resample, and window). This PR changes the error slightly from
to
in order to make it consistent (and I think more helpful, in the case of multiple columns) with the error message from transform.